fix: table toolbar, header columns, click-to-select, wrap and auto-fit (#556) - #557

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

Reviewed the whole branch against the source. The toolbar consolidation, header columns, click-to-select and double-click auto-fit all hold up, and the mark-aware wrapRuns is a careful piece of work — keeping the original whitespace character instead of reconstructing it is the right call. One confirmed defect before this can merge.

The sizing wrap and the render wrap are not the same algorithm

richText.js says wrapCharLine is the "same algorithm as textMetrics.js's wrapLines, so a cell wraps exactly where the height that was measured for it expects it to". They diverge on consecutive whitespace, and the render always produces the extra line:

• wrapLines splits on /\s+/ and drops empties, so a run of whitespace costs one column.
• wrapCharLine keeps every original whitespace character as its own token, so a run of n whitespace characters costs n columns.

The sizing path — wrappedCellLines → wrappedCellHeight, which feeds growTableRow, flushDraft and autoFitRowHeight — goes through wrapLines. The canvas render (wrappedCellRunLines) and the export (useThumbnail.js) go through wrapRuns. So a cell holding two adjacent spaces is grown for fewer lines than it is drawn with, and the last line falls outside the row.

Reproduces on a default table (TABLE_CELL_W 120, TABLE_FONT_SIZE 14, so perLine is 12):

"Total:  1,240"
  row grown for 1 line : ["Total: 1,240"]
  canvas draws   2 lines: ["Total:  ", "1,240"]

"one   two   three   four"   (perLine 20)
  row grown for 1 line : ["one two three four"]
  canvas draws   2 lines: ["one   two   three   ", "four"]

Two spaces after a full stop is ordinary typing, so this is reachable without anything unusual in the cell. trimRuns does not help — it only trims the ends, and the divergence is interior.

The intent behind wrapCharLine is right, so the fix probably belongs on the sizing side: measure with the same per-character tokenizer rather than with wrapLines, so both paths count whitespace the same way. Then the comment's claim becomes true, and the export, the canvas and the row height agree by construction rather than by coincidence.

Worth a test that pins the two paths against each other over text with double spaces, so they cannot drift apart again.

Minor

The known gap you flagged in the commit message — clicking empty canvas does not deselect an open table on a unified document — has no issue filed for it that I can find. Please file it so it is not lost.

Verified, no action needed

• ItemListRow and Popover's toggle default-slot prop both exist in frappe-ui 1.0.0-beta.19.
• border in WhiteboardObjectGroup cannot be undefined: hasSelection is cells.length > 0, so firstCell is always present when it is true.
• charsPerLine clamps to 1, so a 24px column cannot drive wrapCharLine's long-word loop into a hang.
• No unused imports left after the refactor.

The wrap defect is fixed at the right layer. wrappedCellLines now goes through wrapRuns' per-character tokenizer, so sizing and rendering share one definition of what a run of whitespace costs instead of two that disagreed — wrapLines' /\s+/ charging one column where the render charged one per character. Sizing and drawing now agree by construction rather than by coincidence, which is the fix I was after.

The new test compares the two paths directly on the repro ('one two three four' at 120px), so it fails if they ever diverge again. The comments in both files were updated to describe the new arrangement rather than left pointing at the old one.

Merging.